Skip to content

Enable inlay hints for the supporting servers#843

Merged
feoh merged 2 commits into
nvim-lua:masterfrom
VlaDexa:inlay-hints
Apr 17, 2024
Merged

Enable inlay hints for the supporting servers#843
feoh merged 2 commits into
nvim-lua:masterfrom
VlaDexa:inlay-hints

Conversation

@VlaDexa

@VlaDexa VlaDexa commented Apr 14, 2024

Copy link
Copy Markdown
Contributor

Since latest neovim nightly supports inlay hints maybe you should look into enabling them by default, like vscode does.

This can be an unwanted change for some people, since the implemented hints are in the middle of the code and I know some people that don't like that.

Maybe that can be left in, but commented out so the users may know that this feature is supported, just not enabled by default.

@WindSoilder

WindSoilder commented Apr 15, 2024

Copy link
Copy Markdown

I think it's good to have a way to toggle inlay hint, because it can be too verbosely.

This is how I do on my config:

if client.server_capabilities.inlayHintProvider then
    map('<leader>ih', function() vim.lsp.inlay_hint.enable(0, not vim.lsp.inlay_hint.is_enabled()) end, "Toggle [I]nlay [H]int")
end

@feoh

feoh commented Apr 15, 2024

Copy link
Copy Markdown
Collaborator

Agreed. A toggle or configurable is needed here.

@dam9000

dam9000 commented Apr 15, 2024

Copy link
Copy Markdown
Contributor

If a toggle is added I would suggest to use <leader>ti

@VlaDexa

VlaDexa commented Apr 15, 2024

Copy link
Copy Markdown
Contributor Author

Maybe since this affects code stuff and uses LSP it should be <leader>ci?

@feoh

feoh commented Apr 17, 2024

Copy link
Copy Markdown
Collaborator

@VlaDexa Thanks for your work thus far. If you're still intrerested in getting this merged, please add a toggle as folks have detailed. I don't honestly care which key bind you use. People can always change it :)

@WindSoilder

WindSoilder commented Apr 17, 2024

Copy link
Copy Markdown

Sorry to have more thought here: I don't think <leader>ci match the command naming style in kickstart, because c doesn't show in help message.

Here are some examples of how key binds to help messages:

map('<leader>ca', vim.lsp.buf.code_action, '[C]ode [A]ction')
map('<leader>rn', vim.lsp.buf.rename, '[R]e[n]ame')
map('<leader>ds', require('telescope.builtin').lsp_document_symbols, '[D]ocument [S]ymbols')

Every character has a highlight in help message(which wrapped with []). It's good for users to remember key bind. So I'd suggest to change keybind to <leader>it, or changing help message to [C]hange [I]nlay hint

@feoh

feoh commented Apr 17, 2024

Copy link
Copy Markdown
Collaborator

More thoughts are good so long as the contributor of said thoughts helps find concordance and we can move forward, assuming folks agree that we all want this optional feature.

@dam9000

dam9000 commented Apr 17, 2024

Copy link
Copy Markdown
Contributor

<leader>ti [T]oggle [I]nlay still makes the most sense to me :)

@VlaDexa

VlaDexa commented Apr 17, 2024

Copy link
Copy Markdown
Contributor Author

If we're going by the starting letters of the help message logic, then sure, I agree that ci is not conforming. Then, I think that it being <leader>th is easier to remember, since you don't need to know that it toggles something inlay, but enables hints.

@dam9000

dam9000 commented Apr 17, 2024

Copy link
Copy Markdown
Contributor

<leader>th is fine with me 👍

Comment thread init.lua Outdated
@feoh feoh merged commit 5540527 into nvim-lua:master Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants